-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(inputs.http_response): Fix for IPv4 and IPv6 addresses when interface is set #15496
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @zak-pawel! Just a few small comments...
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zak-pawel!
} | ||
} | ||
|
||
return nil, fmt.Errorf("cannot create local address for interface %q and server address %q", interfaceName, address.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to break the case when the address is a hostname (in my case pointing to an ipv6 address) instead of an ip address:
Error running agent: could not initialize input inputs.http_response: cannot create local address for interface "tinc.retiolum" and server address "http://transmission.r/transmission/web/"
Summary
This PR solves the problem that existed in the scenario where the
urls
list contains mix of IPv4 and IPv6 addresses and theinterface
is config file set.Until now, the plugin chooses the first available address for the given interface. In my case, it was IPv6 for Windows and IPv4 for Linux. Because of this, the plugin worked incorrectly with IPv4 addresses for Windows and IPv6 addresses for Linux.
Here is a list of the most important things done in this PR:
Each
URL
now has its ownhttpClient
(until now, onehttpClient
was used to handle allURLs
).Each
URL
provided in the config and each address for the giveninterface
in the config is parsed and checked to see if it is an IPv4 or IPv6 address.url.URL
andnet.IPNet
. That's why I added github.com/seancfoley/ipaddress-go lib. The topic of parsing addresses is complex enough that I didn't see the point in doing it myself. If any of the reviewers have an idea on how to do this better, I would be happy to hear it :)Each
URL
has ahttpClient
assigned that matches the address format (IPv4 or IPv6).Configuration checking and initializing defaults have been moved from
Gather
toInit
.h.ResponseStringMatch
failsURLs
provided in the config cannot be parsedURLs
provided in the config is not of typehttp
orhttps
httpClient
for theURL
(previously, these errors could occur in
Gather
, which did not cause the plugin to fail, but rather resulted in recurring errors in the logs)Thanks to this PR, the
http_response
plugin tests forWindows
are re-enabled.Checklist
Related issues
resolves #8451